Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update wasm build to use bigint #1566

Closed

Conversation

nonameentername
Copy link

@nonameentername nonameentername commented Aug 28, 2024

godot-cpp does not use the same compilation flags for wasm as godot. This change fixes import errors from mismatched function signatures.

Fixes godotengine/godot#96492
Fixes #1553

godot-cpp does not use the same compilation flags for wasm as godot.
This change fixes import errors from mismatched function signatures.
@nonameentername nonameentername requested a review from a team as a code owner August 28, 2024 23:42
tools/web.py Outdated
Comment on lines 50 to 51
env.Append(CCFLAGS=["-sWASM_BIGINT"])
env.Append(LINKFLAGS=["-sWASM_BIGINT"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the main repo I could only find this flag appended to LINKFLAGS:

https://github.com/godotengine/godot/blob/ce8a837aab2568f9cdc41b4b410c478b0cd711fc/platform/web/detect.py#L275-L276

And only when the emscripten version is greater than 3.1.41, or when proxy_to_pthread is enabled.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll update pr to do the same.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
env.Append(CCFLAGS=["-sWASM_BIGINT"])
env.Append(LINKFLAGS=["-sWASM_BIGINT"])
env.Append(LINKFLAGS=["-sWASM_BIGINT"])

Since we need to build gdextensions with emscripten => 3.1.62 (better if == 3.1.62) to support gdextensions, we don't need to check the compiler version or proxy_to_pthread (in fact, I think we should remove those conditionals in upstream Godot to, and always require emscripten => 3.1.62).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Faless I'll create a PR for this for the main repo.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we're still waiting on the CCFLAGS line to be removed. But once that's done, I think this should be OK to merge!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've opened #1603 because this can't be delayed much longer, since it breaks all web GDExtensions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This has been identified as a bug cherrypick:4.3 platform:web topic:buildsystem Related to the buildsystem or CI setup
Projects
None yet
5 participants